Hmm, that's odd indeed. Does anything break if you change Path.is_in_build_dir instead so that in returns true for Path.build_dir? For the universe file, this fact should come from this check

Does anything break if you change Path.is_in_build_dir instead so that in returns true for Path.build_dir?

Yup, there's breakage if we try to change it:

+  Trying to build _build/.universe-state but build context .universe-state doesn't exist.

It comes from this check starting to fail for _build/.universe-state https://github.com/rgrinberg/jbuilder/blob/is-in-build-dir-weird-behavior/src/build_system.ml#L1043-L1044

For the universe file, this fact should come from this check

Hmm, it's a bit confusing that the build_dir is assumed to have no targets since clearly .universe-state is clearly one. Though, this isn't really the problem as we don't really have a build rule for the .universe-state file. In the current code, we are simply trying to trick the build system that this file should always exist. But this is the dependent on wonkiness of Path.is_in_build_dir that I've outlined.

It's not really that we don't have a rule for .universe-state. The build system works as follow:

• first we look if we have a rule for a given file in t.files
• if not we try to load the directory of the file first and look again

Loading a directory fills all the corresponding entries in t.files. For special files such as .universe-state, we eagerly fill t.files with the rule. The check in get_dir_status just means that trying to get a listing of the build directory will return an empty set of files.

Regarding this issue, I think we should do one of the following:

• rename Path.is_in_build_dir to make its semantic clearer. For instance Path.is_strict_descendant_of_build_dir
• change the behavior of Path.is_in_build_dir and change the test you pointed

For special files such as .universe-state, we eagerly fill t.files with the rule

Where does this happen for .universe-state? All I see is writing the file.

change the behavior of Path.is_in_build_dir and change the test you pointed

This is option is preferable, but something else needs to change as well here. Just changing it breaks jbuilder in the way I've pointed above.

Ah, sorry, you are right, we just write the file directly.

This is option is preferable, but something else needs to change as well here. Just changing it breaks jbuilder in the way I've pointed above

Agreed

I'm merging this for now to make sure this property doesn't change anywhere. I'll actually do the rename you suggested after #744 and then look for a proper fix.

Ok. BTW, I consider the rename to be a proper fix. The most important is that the code does what it looks like it does.